From: David Brownell <david-b@pacbell.net>
To: linux-usb-devel@lists.sourceforge.net
Subject: [patch 2.6.12-rc2] usb hcd/pci suspend/resume fixes
Date: Tue, 5 Apr 2005 14:00:05 -0700
User-Agent: KMail/1.7.1
Cc: Colin Leroy <colin@colino.net>
References: <20050405204449.5ab0cdea@jack.colino.net>
In-Reply-To: <20050405204449.5ab0cdea@jack.colino.net>
MIME-Version: 1.0
Content-Type: Multipart/Mixed;
  boundary="Boundary-00=_VxvUCPiyUY60lfh"
Message-Id: <200504051400.05582.david-b@pacbell.net>

--Boundary-00=_VxvUCPiyUY60lfh
Content-Type: text/plain;
  charset="us-ascii"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

This is a resend; the patch is in 2.6.12-rc2-mm1 already.

- Dave

--Boundary-00=_VxvUCPiyUY60lfh
Content-Type: text/x-diff;
  charset="us-ascii";
  name="susp-0330a.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename="susp-0330a.patch"

This has a variety of updates to the shared suspend/resume code for
PCI based USB host controllers.

    - Cope with pm_message_t replacing the target system state.
      This is actually a loss of functionality; PCI D1 and D2
      states will no longer be used, and it's no longer knowable
      that D3cold is on the way so power will be lost.

    - Most importantly, some of the resume paths are reworked and
      cleaned up.  They're now an exact mirror of suspend paths,
      and more care is taken to ensure the hardware is reactivated
      before the hardware re-enables interrupts.

Plus comment and diagnostic cleanups; there are some nasty cases here 
especially combined with swsusp, now they're somewhat commented.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Andrew Morton <akpm@osdl.org>

 25-akpm/drivers/usb/core/hcd-pci.c |  151 ++++++++++++++++++++++---------------
 1 files changed, 91 insertions(+), 60 deletions(-)

--- 25/drivers/usb/core/hcd-pci.c~usb-resume-fixes	Wed Mar 30 13:26:29 2005
+++ 25-akpm/drivers/usb/core/hcd-pci.c	Wed Mar 30 13:26:29 2005
@@ -33,7 +33,7 @@
 #include "hcd.h"
 
 
-/* PCI-based HCs are normal, but custom bus glue should be ok */
+/* PCI-based HCs are common, but plenty of non-PCI HCs are used too */
 
 
 /*-------------------------------------------------------------------------*/
@@ -67,8 +67,8 @@ int usb_hcd_pci_probe (struct pci_dev *d
 
 	if (pci_enable_device (dev) < 0)
 		return -ENODEV;
-	dev->current_state = 0;
-	dev->dev.power.power_state = 0;
+	dev->current_state = PCI_D0;
+	dev->dev.power.power_state = PMSG_ON;
 	
         if (!dev->irq) {
         	dev_err (&dev->dev,
@@ -186,26 +186,14 @@ EXPORT_SYMBOL (usb_hcd_pci_remove);
 
 #ifdef	CONFIG_PM
 
-static char __attribute_used__ *pci_state(u32 state)
-{
-	switch (state) {
-	case 0:		return "D0";
-	case 1:		return "D1";
-	case 2:		return "D2";
-	case 3:		return "D3hot";
-	case 4:		return "D3cold";
-	}
-	return NULL;
-}
-
 /**
  * usb_hcd_pci_suspend - power management suspend of a PCI-based HCD
  * @dev: USB Host Controller being suspended
- * @state: state that the controller is going into
+ * @message: semantics in flux
  *
  * Store this function in the HCD's struct pci_driver as suspend().
  */
-int usb_hcd_pci_suspend (struct pci_dev *dev, u32 state)
+int usb_hcd_pci_suspend (struct pci_dev *dev, pm_message_t message)
 {
 	struct usb_hcd		*hcd;
 	int			retval = 0;
@@ -213,13 +201,23 @@ int usb_hcd_pci_suspend (struct pci_dev 
 
 	hcd = pci_get_drvdata(dev);
 
+	/* FIXME until the generic PM interfaces change a lot more, this
+	 * can't use PCI D1 and D2 states.  For example, the confusion
+	 * between messages and states will need to vanish, and messages
+	 * will need to provide a target system state again.
+	 *
+	 * It'll be important to learn characteristics of the target state,
+	 * especially on embedded hardware where the HCD will often be in
+	 * charge of an external VBUS power supply and one or more clocks.
+	 * Some target system states will leave them active; others won't.
+	 * (With PCI, that's often handled by platform BIOS code.)
+	 */
+
 	/* even when the PCI layer rejects some of the PCI calls
 	 * below, HCs can try global suspend and reduce DMA traffic.
 	 * PM-sensitive HCDs may already have done this.
 	 */
 	has_pci_pm = pci_find_capability(dev, PCI_CAP_ID_PM);
-	if (state > 4)
-		state = 4;
 
 	switch (hcd->state) {
 
@@ -228,7 +226,7 @@ int usb_hcd_pci_suspend (struct pci_dev 
 	 */
 	case HC_STATE_RUNNING:
 		hcd->state = HC_STATE_QUIESCING;
-		retval = hcd->driver->suspend (hcd, state);
+		retval = hcd->driver->suspend (hcd, message);
 		if (retval) {
 			dev_dbg (hcd->self.controller, 
 					"suspend fail, retval %d\n",
@@ -246,14 +244,11 @@ int usb_hcd_pci_suspend (struct pci_dev 
 	 * have been called, otherwise root hub timers still run ...
 	 */
 	case HC_STATE_SUSPENDED:
-		if (state <= dev->current_state)
-			break;
-
-		/* no DMA or IRQs except in D0 */
-		if (!dev->current_state) {
+		/* no DMA or IRQs except when HC is active */
+		if (dev->current_state == PCI_D0) {
+			free_irq (hcd->irq, hcd);
 			pci_save_state (dev);
 			pci_disable_device (dev);
-			free_irq (hcd->irq, hcd);
 		}
 
 		if (!has_pci_pm) {
@@ -261,25 +256,19 @@ int usb_hcd_pci_suspend (struct pci_dev 
 			break;
 		}
 
-		/* POLICY: ignore D1/D2/D3hot differences;
-		 * we know D3hot will always work.
+		/* NOTE:  dev->current_state becomes nonzero only here, and
+		 * only for devices that support PCI PM.  Also, exiting
+		 * PCI_D3 (but not PCI_D1 or PCI_D2) is allowed to reset
+		 * some device state (e.g. as part of clock reinit).
 		 */
-		retval = pci_set_power_state (dev, state);
-		if (retval < 0 && state < 3) {
-			retval = pci_set_power_state (dev, 3);
-			if (retval == 0)
-				state = 3;
-		}
+		retval = pci_set_power_state (dev, PCI_D3hot);
 		if (retval == 0) {
-			dev_dbg (hcd->self.controller, "--> PCI %s\n",
-					pci_state(dev->current_state));
-#ifdef	CONFIG_USB_SUSPEND
-			pci_enable_wake (dev, state, hcd->remote_wakeup);
-			pci_enable_wake (dev, 4, hcd->remote_wakeup);
-#endif
+			dev_dbg (hcd->self.controller, "--> PCI D3\n");
+			pci_enable_wake (dev, PCI_D3hot, hcd->remote_wakeup);
+			pci_enable_wake (dev, PCI_D3cold, hcd->remote_wakeup);
 		} else if (retval < 0) {
-			dev_dbg (&dev->dev, "PCI %s suspend fail, %d\n",
-					pci_state(state), retval);
+			dev_dbg (&dev->dev, "PCI D3 suspend fail, %d\n",
+					retval);
 			(void) usb_hcd_pci_resume (dev);
 			break;
 		}
@@ -287,13 +276,14 @@ int usb_hcd_pci_suspend (struct pci_dev 
 	default:
 		dev_dbg (hcd->self.controller, "hcd state %d; not suspended\n",
 			hcd->state);
+		WARN_ON(1);
 		retval = -EINVAL;
 		break;
 	}
 
 	/* update power_state **ONLY** to make sysfs happier */
 	if (retval == 0)
-		dev->dev.power.power_state = state;
+		dev->dev.power.power_state = message;
 	return retval;
 }
 EXPORT_SYMBOL (usb_hcd_pci_suspend);
@@ -308,7 +298,6 @@ int usb_hcd_pci_resume (struct pci_dev *
 {
 	struct usb_hcd		*hcd;
 	int			retval;
-	int			has_pci_pm;
 
 	hcd = pci_get_drvdata(dev);
 	if (hcd->state != HC_STATE_SUSPENDED) {
@@ -316,31 +305,73 @@ int usb_hcd_pci_resume (struct pci_dev *
 				"can't resume, not suspended!\n");
 		return 0;
 	}
-	has_pci_pm = pci_find_capability(dev, PCI_CAP_ID_PM);
 
-	/* D3cold resume isn't usually reported this way... */
-	dev_dbg(hcd->self.controller, "resume from PCI %s%s\n",
-			pci_state(dev->current_state),
-			has_pci_pm ? "" : " (legacy)");
+	/* NOTE:  chip docs cover clean "real suspend" cases (what Linux
+	 * calls "standby", "suspend to RAM", and so on).  There are also
+	 * dirty cases when swsusp fakes a suspend in "shutdown" mode.
+	 */
+	if (dev->current_state != PCI_D0) {
+#ifdef	DEBUG
+		int	pci_pm;
+		u16	pmcr;
+
+		pci_pm = pci_find_capability(dev, PCI_CAP_ID_PM);
+		pci_read_config_word(dev, pci_pm + PCI_PM_CTRL, &pmcr);
+		pmcr &= PCI_PM_CTRL_STATE_MASK;
+		if (pmcr) {
+			/* Clean case:  power to USB and to HC registers was
+			 * maintained; remote wakeup is easy.
+			 */
+			dev_dbg(hcd->self.controller, "resume from PCI D%d\n",
+					pmcr);
+		} else {
+			/* Clean:  HC lost Vcc power, D0 uninitialized
+			 *   + Vaux may have preserved port and transceiver
+			 *     state ... for remote wakeup from D3cold
+			 *   + or not; HCD must reinit + re-enumerate
+			 *
+			 * Dirty: D0 semi-initialized cases with swsusp
+			 *   + after BIOS init
+			 *   + after Linux init (HCD statically linked)
+			 */
+			dev_dbg(hcd->self.controller,
+				"PCI D0, from previous PCI D%d\n",
+				dev->current_state);
+		}
+#endif
+		pci_enable_wake (dev, dev->current_state, 0);
+		pci_enable_wake (dev, PCI_D3cold, 0);
+	} else {
+		/* Same basic cases: clean (powered/not), dirty */
+		dev_dbg(hcd->self.controller, "PCI legacy resume\n");
+	}
 
-	hcd->state = HC_STATE_RESUMING;
+	/* NOTE:  the PCI API itself is asymmetric here.  We don't need to
+	 * pci_set_power_state(PCI_D0) since that's part of re-enabling;
+	 * but that won't re-enable bus mastering.  Yet pci_disable_device()
+	 * explicitly disables bus mastering...
+	 */
+	retval = pci_enable_device (dev);
+	if (retval < 0) {
+		dev_err (hcd->self.controller,
+			"can't re-enable after resume, %d!\n", retval);
+		return retval;
+	}
+	pci_set_master (dev);
+	pci_restore_state (dev);
 
-	if (has_pci_pm)
-		pci_set_power_state (dev, 0);
-	dev->dev.power.power_state = 0;
+	dev->dev.power.power_state = PMSG_ON;
+
+	hcd->state = HC_STATE_RESUMING;
+	hcd->saw_irq = 0;
 	retval = request_irq (dev->irq, usb_hcd_irq, SA_SHIRQ,
-				hcd->driver->description, hcd);
+				hcd->irq_descr, hcd);
 	if (retval < 0) {
 		dev_err (hcd->self.controller,
 			"can't restore IRQ after resume!\n");
+		usb_hc_died (hcd);
 		return retval;
 	}
-	hcd->saw_irq = 0;
-	pci_restore_state (dev);
-#ifdef	CONFIG_USB_SUSPEND
-	pci_enable_wake (dev, dev->current_state, 0);
-	pci_enable_wake (dev, 4, 0);
-#endif
 
 	retval = hcd->driver->resume (hcd);
 	if (!HC_IS_RUNNING (hcd->state)) {
_


--Boundary-00=_VxvUCPiyUY60lfh--